ValidationError messages include field value#368
Conversation
|
The proposed change breaks some test cases. Can you run |
There was a problem hiding this comment.
@bitmage , I think this shouldn't be logged by default to server console.
There was a problem hiding this comment.
Yes, I will remove this.
|
Is the format of the message good? I don't want to waste time correcting expectations in a dozen tests until I get confirmation on that. |
|
My two cents: the proposed format does not distinguish between different types, it's hard to tell whether the value was a string or a number. It also does not work for object types and when the property value is large (e.g. a 1kb text), the error message will be unreadable. I am proposing:
|
|
What's the status of this work? Are you @bitmage still willing to finish it? |
|
Oh yeah, sorry! I will take a look in the next day or two and try to get things wrapped up. |
451c6fa to
385a0eb
Compare
There was a problem hiding this comment.
What do you think of this? I noticed we were producing some strange output in the case of nested documents (which have a type of function). I don't know how to output a sensible output in that case... but I think it's an edge case that can be left for another day.
For strings, objects, and undefined/null we now have some pretty clear output.
There was a problem hiding this comment.
Handling non-scalar values (objects, arrays, functions) will be always tricky. You can take a look at chai.js which basically prints only top-level keys of the object.
As for function values, JSON.stringify removes them from the output, which solves the problem. On the other hand, it does not handle circular references, which may be a problem.
|
Your Model prototype is implementing the inspect method and configuring it to output colors. Any time a model is console.log'd or inspected it will run this method. This overrides whatever top-level configuration is used, so I can fall back to JSON.stringify as you originally suggested, but it's a blocking operation, and I'm still concerned about the lack of a depth limit. |
|
Closing in favour of #389. |
Good catch! There are several ways how to fix the problem, e.g. by passing |
|
On Node v0.11.14+, one can return an object from the custom inspect function. That way all options are honoured. For Node v0.10 and older, I have disabled color output. |
5 participants
I modified the formatErrors() function to include the current value of the fields which have failed validation.
The format looks like this:
Please let me know if that format is good, and I will update the test expectations to match.